SPLAT-2253: CCM-AWS config enforce to provision Service NLB with SG under gate#391
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
mtulio
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@mtulio: This pull request references SPLAT-2253 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/test all |
mtulio
changed the title
|
PR rebased with upstream updates, and CCCMO FG support by #400 |
|
Next step: create a CI job to exercise this scenario. |
|
/test ? |
|
@mtulio: The following commands are available to trigger required jobs: The following commands are available to trigger optional jobs: Use DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@mtulio: This pull request references SPLAT-2253 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
mtulio
changed the title
openshift-ci-robot
removed
the
jira/valid-reference
|
@mtulio: No Jira issue is referenced in the title of this pull request. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/payload-job ? |
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws-ovn openshift/origin#30235 openshift/cloud-provider-aws#117 |
|
/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws-ovn openshift/cloud-provider-aws#117 |
|
/assign @theobarberbany |
|
Removing the hold as this PR is already ready. /hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
| } | ||
| if isFeatureGateEnabled(features, "AWSServiceLBNetworkSecurityGroup") { | ||
| if cfg.Global.NLBSecurityGroupMode != awsconfig.NLBSecurityGroupModeManaged { | ||
| // OpenShift enforces to CCM manage security group by default when deploying |
There was a problem hiding this comment.
Can we clarify this a little please? By default OCP has the CCM manage security groups upon deployment?
There was a problem hiding this comment.
Hey @theobarberbany . No, by default CCM-AWS does not attach SGs to Service type-loadBalancer NLB, only CLB (default lb type). To configure CCM to managed SG for NLB we need to enable that configuration. I will update the comment.
There was a problem hiding this comment.
Ah thanks! Makes more sense to me now :)
theobarberbany
left a comment
theobarberbany
left a comment
There was a problem hiding this comment.
one non blocking nit with a comment's wording, otherwise
/lgtm
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: theobarberbany The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/hold in case you want to fix the nit |
openshift-ci
bot
added
the
do-not-merge/hold
Enforce CCM to manage Security Group by default for security compliance and best practices on Service type-loadBalancer when using Network Load Balancer (NLB). Fixes INI files with sections sorted co-authored by Claude. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Marco Braga <mrbraga@redhat.com>
openshift-ci-robot
removed
the
verified
Hey @theobarberbany , good suggestion. Fixed! |
|
Already verified, label removed after updating code comment. /verified by @huali9 on comment #391 (comment) |
openshift-ci-robot
added
the
verified
|
@mtulio: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/lgtm |
|
/retest-required |
|
@mtulio: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Nit/comment fixed, /hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
openshift-merge-bot
bot
merged commit 6148c0c
into
openshift:main
8 participants
Updating the k/cloud-provider-aws to gather the feature of Service type-loadBalancer NLB with managed Security Group through cloud-config under the OpenShift feature set
TechPreviewNoUpgrade.Upstream feature:
https://github.com/kubernetes/cloud-provider-aws/pull/1158